Support auto-setting AWS credentials for storage options #78

milesgranger · 2024-03-15T11:06:35Z

Will close #72

Also fix #74 along the way.

In [1]: import dask_deltatable as ddt
   ...: import dask.dataframe as dd
   ...: import pandas as pd
   ...:
   ...: df = pd.DataFrame({"x": range(100), "y": pd.Series(range(100)) // 2})
   ...: ddf = dd.from_pandas(df, npartitions=4)
   ...:
   ...: ddt.to_deltalake('s3://dask-deltatable-ci/test-delta', ddf)
   ...: read = ddt.read_deltalake('s3://dask-deltatable-ci/test-delta')

In [2]: read
Out[2]:
Dask DataFrame Structure:
                   x      y
npartitions=4
               int64  int64
                 ...    ...
                 ...    ...
                 ...    ...
                 ...    ...
Dask Name: to_string_dtype, 2 expressions
Expr=ArrowStringConversion(frame=FromMapProjectable(cdd6cff))

@mrocklin, I haven't added tests for this yet. I'm curious if you prefer integration type tests doing actual S3 writes to a protected bucket, so we could add some limited credentials here for like the s3://dask-deltalake-ci bucket. Or if you think mocking it is okay.

codecov-commenter · 2024-03-15T11:18:14Z

Codecov Report

Attention: Patch coverage is 52.63158% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 71.62%. Comparing base (cbe085a) to head (14e4c93).
Report is 1 commits behind head on main.

Files	Patch %	Lines
dask_deltatable/utils.py	26.08%	17 Missing ⚠️
dask_deltatable/core.py	85.71%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #78      +/-   ##
==========================================
- Coverage   74.06%   71.62%   -2.44%     
==========================================
  Files           6        6              
  Lines         320      356      +36     
==========================================
+ Hits          237      255      +18     
- Misses         83      101      +18

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mrocklin · 2024-03-15T12:55:18Z

No personal preference on testing. Deferring to you all

…

On Fri, Mar 15, 2024 at 6:18 AM Codecov Comments Bot < ***@***.***> wrote: Codecov <https://app.codecov.io/gh/dask-contrib/dask-deltatable/pull/78?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dask-contrib> Report Attention: Patch coverage is 52.63158% with 18 lines in your changes are missing coverage. Please review. Project coverage is 71.62%. Comparing base (cbe085a) <https://app.codecov.io/gh/dask-contrib/dask-deltatable/commit/cbe085a9a7acc66be782024d64df84abc175995e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dask-contrib> to head (14e4c93) <https://app.codecov.io/gh/dask-contrib/dask-deltatable/pull/78?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dask-contrib> . Report is 1 commits behind head on main. Files <https://app.codecov.io/gh/dask-contrib/dask-deltatable/pull/78?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dask-contrib> Patch % Lines dask_deltatable/utils.py <https://app.codecov.io/gh/dask-contrib/dask-deltatable/pull/78?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dask-contrib#diff-ZGFza19kZWx0YXRhYmxlL3V0aWxzLnB5> 26.08% 17 Missing ⚠️ <https://app.codecov.io/gh/dask-contrib/dask-deltatable/pull/78?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dask-contrib> dask_deltatable/core.py <https://app.codecov.io/gh/dask-contrib/dask-deltatable/pull/78?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dask-contrib#diff-ZGFza19kZWx0YXRhYmxlL2NvcmUucHk=> 85.71% 1 Missing ⚠️ <https://app.codecov.io/gh/dask-contrib/dask-deltatable/pull/78?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dask-contrib> ❗ Your organization needs to install the Codecov GitHub app <https://github.com/apps/codecov/installations/select_target> to enable full functionality. Additional details and impacted files @@ Coverage Diff @@## main #78 +/- ## ==========================================- Coverage 74.06% 71.62% -2.44% ========================================== Files 6 6 Lines 320 356 +36 ==========================================+ Hits 237 255 +18 - Misses 83 101 +18 ☔ View full report in Codecov by Sentry <https://app.codecov.io/gh/dask-contrib/dask-deltatable/pull/78?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dask-contrib> . 📢 Have feedback on the report? Share it here <https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dask-contrib> . — Reply to this email directly, view it on GitHub <#78 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACKZTGGWX5KDGX6QYL3XFTYYLKIZAVCNFSM6AAAAABEXZLOBOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJZGQ2DINZZGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

jacobtomlinson

It's probably best if tests don't rely on external resources. They'll just end up brittle or skipped for most folks. Given how well defined the S3 API is I think mocking is a good approach here.

Support auto-setting AWS credentials for storage options

14e4c93

milesgranger force-pushed the milesgranger/72-auto-provide-s3-creds branch from cfbc44f to 14e4c93 Compare March 15, 2024 11:16

milesgranger self-assigned this Mar 15, 2024

jacobtomlinson reviewed Mar 15, 2024

View reviewed changes

milesgranger added 2 commits March 18, 2024 10:38

Tests for get_bucket_region and maybe_set_aws_credentials

5261ee0

Tests to ensure to/read_deltalake call maybe_set_aws_credentails

b58712e

milesgranger requested a review from jacobtomlinson March 18, 2024 10:08

Skipif boto3 not installed

44b9a3b

milesgranger requested a review from jrbourbeau March 20, 2024 11:49

fjetter approved these changes Mar 21, 2024

View reviewed changes

fjetter merged commit ec1c90c into dask-contrib:main Mar 21, 2024
10 checks passed

milesgranger deleted the milesgranger/72-auto-provide-s3-creds branch March 21, 2024 08:12

milesgranger mentioned this pull request Mar 21, 2024

Fix up some mypy errors #76

Merged

jacobtomlinson mentioned this pull request Apr 29, 2024

read_deltalake breaks with dask>=2024.3.1 #79

Closed

avriiil mentioned this pull request May 20, 2024

ImportError with deltalake=0.16.0 #74

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support auto-setting AWS credentials for storage options #78

Support auto-setting AWS credentials for storage options #78

milesgranger commented Mar 15, 2024

codecov-commenter commented Mar 15, 2024

mrocklin commented Mar 15, 2024 via email

jacobtomlinson left a comment

Support auto-setting AWS credentials for storage options #78

Support auto-setting AWS credentials for storage options #78

Conversation

milesgranger commented Mar 15, 2024

codecov-commenter commented Mar 15, 2024

Codecov Report

mrocklin commented Mar 15, 2024 via email

jacobtomlinson left a comment

Choose a reason for hiding this comment